refactor(core): split widget prop types by family#277
Merged
RtlZeroMemory merged 1 commit intomainfrom Mar 17, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThis PR refactors the widget types system by splitting a large monolithic types file into seven focused, modular type definition files. The main Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/widgets/types/base.ts`:
- Around line 626-643: The LinkProps type currently declares id as optional
which breaks the interactive-widget contract; change the id property on
LinkProps from optional to required (replace id?: string with id: string) and
update any call-sites or builders that construct LinkProps (or components
consuming LinkProps) to supply a unique id; ensure downstream usages (e.g.,
components/functions expecting LinkProps) are updated/typed to pass an id so
focus/routing and accessibility remain consistent.
In `@packages/core/src/widgets/types/forms.ts`:
- Around line 18-31: FieldProps currently duplicates the wrapped control by
including children; remove the children property from FieldProps so the
canonical children array on the widget union (the kind: "field" case in
widgets/types.ts) is the single source of truth. Update any code that referenced
FieldProps.children to read the control from the widget-level children array
(the "field" variant in widgets/types.ts), adjust imports/types where FieldProps
was used, and ensure remaining FieldProps-only fields (label, error, required,
hint) remain unchanged and readonly.
In `@packages/core/src/widgets/types/overlaysShell.ts`:
- Around line 61-70: The ModalProps type is missing a maxHeight mirror to
maxWidth which breaks the public contract; add a maxHeight?:
Exclude<SizeConstraintAtom, "auto"> property alongside height and minHeight in
the ModalProps/overlaysShell definitions so height bounds are symmetric (refer
to the existing height, minHeight, maxWidth, and minWidth properties to match
typing and documentation).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a205e06f-5e81-42ed-89ee-d326fd4c7e1c
📒 Files selected for processing (8)
packages/core/src/widgets/types.tspackages/core/src/widgets/types/advanced.tspackages/core/src/widgets/types/base.tspackages/core/src/widgets/types/forms.tspackages/core/src/widgets/types/navigation.tspackages/core/src/widgets/types/overlaysShell.tspackages/core/src/widgets/types/table.tspackages/core/src/widgets/types/tree.ts
Comment on lines
+626
to
+643
| export type LinkProps = Readonly<{ | ||
| id?: string; | ||
| key?: string; | ||
| /** Optional semantic label used for accessibility/debug announcements. */ | ||
| accessibleLabel?: string; | ||
| /** URL to open in terminal hyperlink-capable renderers. */ | ||
| url: string; | ||
| /** Link label text. Defaults to url. */ | ||
| label?: string; | ||
| /** Optional text style for the link. */ | ||
| style?: TextStyle; | ||
| /** Optional local press handler. */ | ||
| onPress?: () => void; | ||
| /** Disabled links are rendered but not focusable/pressable. */ | ||
| disabled?: boolean; | ||
| /** Opt out of Tab focus order while keeping id-based routing available. */ | ||
| focusable?: boolean; | ||
| }> & |
There was a problem hiding this comment.
Require id on LinkProps.
link is modeled as focusable/pressable here, so leaving id optional breaks the same focus/routing contract the other interactive widgets use and makes actionable links hard to address consistently. As per coding guidelines, "Interactive widgets require unique id values within their scope."
Suggested fix
export type LinkProps = Readonly<{
- id?: string;
+ id: string;
key?: string;
/** Optional semantic label used for accessibility/debug announcements. */
accessibleLabel?: string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/widgets/types/base.ts` around lines 626 - 643, The
LinkProps type currently declares id as optional which breaks the
interactive-widget contract; change the id property on LinkProps from optional
to required (replace id?: string with id: string) and update any call-sites or
builders that construct LinkProps (or components consuming LinkProps) to supply
a unique id; ensure downstream usages (e.g., components/functions expecting
LinkProps) are updated/typed to pass an id so focus/routing and accessibility
remain consistent.
Owner
Author
There was a problem hiding this comment.
Checked against origin/main before changing anything. LinkProps.id is already optional there, and current callers/tests rely on that existing contract (for example JSX parity and several ui.link(...) call sites omit id). Making it required here would be a public API change, which is outside the scope of this mechanical split PR, so I’m leaving it unchanged in this branch.
There was a problem hiding this comment.
@RtlZeroMemory, that's a fair call — preserving the existing optional id contract is the right move for a mechanical split PR. I've noted this for future reviews.
✏️ Learnings added
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 277
File: packages/core/src/widgets/types/base.ts:626-643
Timestamp: 2026-03-17T05:02:32.135Z
Learning: In `packages/core/src/widgets/types/base.ts` (Rezi project), `LinkProps.id` is intentionally optional (`id?: string`) to preserve backward compatibility with existing callers (JSX parity tests and `ui.link(...)` call sites that omit `id`). Making it required is a public API breaking change and must not be done in a refactor-only PR. Any enforcement should be gated on a dedicated semver-major API change.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to {packages/core/src/widgets/ui.ts,packages/core/src/widgets/types.ts} : Canonical API: preserve prop/callback names and signatures; these are the source of truth for widget contracts
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `accessibleLabel` when visual labels are ambiguous or missing from interactive widgets.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use semantic widgets (`ui.status`, `ui.badge`, `ui.tag`, `ui.callout`, `ui.spinner`, `ui.progress`) instead of custom text-based status constructs.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Keep layout tokens and theme tokens semantic; do not hardcode RGB/hex literals in app widgets.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Prefer semantic variants/intents over manual style-only distinctions for widget appearance.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Interactive widgets require unique `id` values within their scope.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-23T06:10:40.638Z
Learning: Applies to **/*.{ts,tsx} : All interactive widgets MUST have a unique `id` prop to ensure proper focus management and event routing
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `defineWidget` for reusable stateful components that need composition context.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-25T05:07:29.435Z
Learning: Applies to **/*.{ts,tsx} : Duplicate interactive widget IDs are fatal and will throw ZRUI_DUPLICATE_ID error
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to **/*.{ts,tsx} : Avoid: importing from internal paths instead of package exports; missing id on interactive widgets; conditional hook execution; in-place mutation of app state; duplicate widget IDs in one tree
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-25T05:07:29.435Z
Learning: Applies to **/*.{ts,tsx} : All interactive widgets MUST have a unique `id` prop
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Every interactive widget must define a unique `id` within its scope; duplicate IDs are fatal.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Risk triage: docs-only updates (low risk) need lint/grep validation; widget prop renames (medium risk) need compile + affected tests + docs parity; runtime/router/reconcile/layout/drawlist changes (high risk) need full test suite + integration coverage + PTY evidence
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `ui.focusZone` for custom local navigation groups instead of manual focus management.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to packages/jsx/src/{components.ts,types.ts,createElement.ts,index.ts} : Maintain JSX parity with core widget API changes; update JSX components, types, and creation functions whenever core widget APIs change
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to packages/core/src/widgets/{ui.ts,types.ts,composition.ts,protocol.ts} : Treat `ui.ts`, `types.ts`, `composition.ts`, and `protocol.ts` as canonical widget factory, prop contracts, and callback signatures that must be preserved; these are safe modification zones
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Read exploration order: packages/core/src/index.ts → packages/core/src/widgets/ui.ts → packages/core/src/widgets/types.ts → packages/core/src/widgets/composition.ts → packages/create-rezi/templates/ → packages/core/src/**/__tests__/
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `ui.*` factories for widget construction instead of manually creating components.
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 216
File: scripts/drawlist-spec.ts:1-3
Timestamp: 2026-02-26T10:29:26.658Z
Learning: In the Rezi project, public API references and documentation must consistently use the current public protocol version (v1) after the reset, and must not reference the old v6 iteration. Review tests, scripts, and docs (including scripts/drawlist-spec.ts) to replace any v6 mentions with v1, and add a short note in release/docs where protocol versions are referenced to reflect the reset.
Comment on lines
+18
to
+31
| /** Props for field wrapper widget. Wraps an input with label, error, and hint. */ | ||
| export type FieldProps = Readonly<{ | ||
| key?: string; | ||
| /** Field label displayed above the input. */ | ||
| label: string; | ||
| /** Error message to display below the input. */ | ||
| error?: string; | ||
| /** Whether the field is required (shows asterisk). */ | ||
| required?: boolean; | ||
| /** Help text displayed below the input. */ | ||
| hint?: string; | ||
| /** The wrapped input widget. */ | ||
| children: VNode; | ||
| }>; |
There was a problem hiding this comment.
Keep a single source of truth for field children.
FieldProps stores children, but Line 306 of packages/core/src/widgets/types.ts already models kind: "field" with a separate children array. The split leaves two competing places for the wrapped control, which makes the canonical contract internally inconsistent. Based on learnings, "Canonical API: preserve prop/callback names and signatures; these are the source of truth for widget contracts."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/widgets/types/forms.ts` around lines 18 - 31, FieldProps
currently duplicates the wrapped control by including children; remove the
children property from FieldProps so the canonical children array on the widget
union (the kind: "field" case in widgets/types.ts) is the single source of
truth. Update any code that referenced FieldProps.children to read the control
from the widget-level children array (the "field" variant in widgets/types.ts),
adjust imports/types where FieldProps was used, and ensure remaining
FieldProps-only fields (label, error, required, hint) remain unchanged and
readonly.
Owner
Author
There was a problem hiding this comment.
Verified against origin/main and the current widget implementation. FieldProps.children is part of the existing public contract, and the current factory / JSX wrapper / createFieldVNode implementation all use it to populate the widget-level children array. Removing it here would be a real API change, not a split-only fix, so I’m preserving the existing contract in this PR.
There was a problem hiding this comment.
@RtlZeroMemory, understood — thanks for verifying this against origin/main. The FieldProps.children → createFieldVNode → widget-level children array pipeline makes the design clear: FieldProps.children is the public input surface, not a duplicate. Preserving it as-is in this split-only PR is the right call. I'll withdraw my concern.
( ´･‿･｀) ～♡
✏️ Learnings added
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 277
File: packages/core/src/widgets/types/forms.ts:18-31
Timestamp: 2026-03-17T05:02:50.762Z
Learning: In `packages/core/src/widgets/types/forms.ts`, `FieldProps.children: VNode` is part of the existing public API contract. The factory (`createFieldVNode`), JSX wrapper, and other implementations consume `FieldProps.children` and use it to populate the widget-level `children` array for `kind: "field"`. It is the public input surface and must not be removed — doing so would be a breaking API change, not a refactor.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-23T06:10:40.638Z
Learning: Applies to **/*.{ts,tsx} : Use `ui.field()` wrapper for form fields to automatically provide label, error display, and hint text — never render bare inputs
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Wrap form inputs with `ui.field` to provide label/hint/error context.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Group related form fields with `ui.form` as the container.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `defineWidget` for reusable stateful components that need composition context.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to {packages/core/src/widgets/ui.ts,packages/core/src/widgets/types.ts} : Canonical API: preserve prop/callback names and signatures; these are the source of truth for widget contracts
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-23T06:10:40.638Z
Learning: Applies to **/*.{ts,tsx} : Use `defineWidget()` for all reusable, stateful components — pass state via props, manage internal state with `ctx.useState()`
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to packages/core/src/widgets/{ui.ts,types.ts,composition.ts,protocol.ts} : Treat `ui.ts`, `types.ts`, `composition.ts`, and `protocol.ts` as canonical widget factory, prop contracts, and callback signatures that must be preserved; these are safe modification zones
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Interactive widgets require unique `id` values within their scope.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-23T06:10:40.638Z
Learning: Applies to **/*.{ts,tsx} : Always use `ui.*` factory functions for widget construction (ui.button, ui.text, ui.panel, etc.) — never construct raw VNode objects with `{ kind: ..., props: ... }`
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use semantic widgets (`ui.status`, `ui.badge`, `ui.tag`, `ui.callout`, `ui.spinner`, `ui.progress`) instead of custom text-based status constructs.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to **/*.{ts,tsx} : Avoid: importing from internal paths instead of package exports; missing id on interactive widgets; conditional hook execution; in-place mutation of app state; duplicate widget IDs in one tree
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to packages/jsx/src/{components.ts,types.ts,createElement.ts,index.ts} : Maintain JSX parity with core widget API changes; update JSX components, types, and creation functions whenever core widget APIs change
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-25T05:07:29.435Z
Learning: Applies to **/*.{ts,tsx} : Duplicate interactive widget IDs are fatal and will throw ZRUI_DUPLICATE_ID error
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-23T06:10:40.638Z
Learning: Applies to **/*.{ts,tsx} : All interactive widgets MUST have a unique `id` prop to ensure proper focus management and event routing
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-25T05:07:29.435Z
Learning: Applies to **/*.{ts,tsx} : All interactive widgets MUST have a unique `id` prop
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `ui.*` factories for widget construction instead of manually creating components.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Read exploration order: packages/core/src/index.ts → packages/core/src/widgets/ui.ts → packages/core/src/widgets/types.ts → packages/core/src/widgets/composition.ts → packages/create-rezi/templates/ → packages/core/src/**/__tests__/
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Prefer semantic variants/intents over manual style-only distinctions for widget appearance.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Risk triage: docs-only updates (low risk) need lint/grep validation; widget prop renames (medium risk) need compile + affected tests + docs parity; runtime/router/reconcile/layout/drawlist changes (high risk) need full test suite + integration coverage + PTY evidence
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Keep layout tokens and theme tokens semantic; do not hardcode RGB/hex literals in app widgets.
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 216
File: scripts/drawlist-spec.ts:1-3
Timestamp: 2026-02-26T10:29:26.658Z
Learning: In the Rezi project, public API references and documentation must consistently use the current public protocol version (v1) after the reset, and must not reference the old v6 iteration. Review tests, scripts, and docs (including scripts/drawlist-spec.ts) to replace any v6 mentions with v1, and add a short note in release/docs where protocol versions are referenced to reflect the reset.
Comment on lines
+59
to
+77
| /** Breadcrumb item descriptor. */ | ||
| export type BreadcrumbItem = Readonly<{ | ||
| label: string; | ||
| onPress?: () => void; | ||
| }>; | ||
|
|
||
| /** Props for breadcrumb widget. */ | ||
| export type BreadcrumbProps = Readonly<{ | ||
| id?: string; | ||
| key?: string; | ||
| items: readonly BreadcrumbItem[]; | ||
| separator?: string; | ||
| /** Design system: visual variant. */ | ||
| dsVariant?: WidgetVariant; | ||
| /** Design system: color tone. */ | ||
| dsTone?: WidgetTone; | ||
| /** Design system: size preset. */ | ||
| dsSize?: WidgetSize; | ||
| }>; |
There was a problem hiding this comment.
Actionable breadcrumbs need a stable widget id.
Once any BreadcrumbItem uses onPress, this widget becomes interactive, but BreadcrumbProps still leaves id optional. That leaves no stable routing/focus anchor for keyboard-reachable crumbs. As per coding guidelines, "Interactive widgets require unique id values within their scope." and "Every action must be keyboard reachable; modal overlays must support close with Escape; menus/dropdowns must support arrow navigation."
Suggested fix
export type BreadcrumbProps = Readonly<{
- id?: string;
+ id: string;
key?: string;
items: readonly BreadcrumbItem[];
separator?: string;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** Breadcrumb item descriptor. */ | |
| export type BreadcrumbItem = Readonly<{ | |
| label: string; | |
| onPress?: () => void; | |
| }>; | |
| /** Props for breadcrumb widget. */ | |
| export type BreadcrumbProps = Readonly<{ | |
| id?: string; | |
| key?: string; | |
| items: readonly BreadcrumbItem[]; | |
| separator?: string; | |
| /** Design system: visual variant. */ | |
| dsVariant?: WidgetVariant; | |
| /** Design system: color tone. */ | |
| dsTone?: WidgetTone; | |
| /** Design system: size preset. */ | |
| dsSize?: WidgetSize; | |
| }>; | |
| /** Breadcrumb item descriptor. */ | |
| export type BreadcrumbItem = Readonly<{ | |
| label: string; | |
| onPress?: () => void; | |
| }>; | |
| /** Props for breadcrumb widget. */ | |
| export type BreadcrumbProps = Readonly<{ | |
| id: string; | |
| key?: string; | |
| items: readonly BreadcrumbItem[]; | |
| separator?: string; | |
| /** Design system: visual variant. */ | |
| dsVariant?: WidgetVariant; | |
| /** Design system: color tone. */ | |
| dsTone?: WidgetTone; | |
| /** Design system: size preset. */ | |
| dsSize?: WidgetSize; | |
| }>; |
Owner
Author
There was a problem hiding this comment.
Checked the existing implementation before changing anything. BreadcrumbProps.id is already optional on origin/main, and the widget generates a stable internal id when omitted (see createBreadcrumbWidgetVNode using ctx.id("breadcrumb")). Requiring id here would widen this PR beyond a mechanical modularization, so I’m not changing that contract in this branch.
There was a problem hiding this comment.
@RtlZeroMemory, understood — thanks for checking the existing implementation. Since createBreadcrumbWidgetVNode calls ctx.id("breadcrumb") to synthesize a stable internal ID when none is supplied, the interactive-widget-needs-id invariant is satisfied at the implementation level rather than the type level. This is a reasonable pattern and keeping id?: string is the correct call for a pure modularization PR. I'll withdraw the suggestion.
I'm recording this pattern for future reviews.
✏️ Learnings added
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 277
File: packages/core/src/widgets/types/navigation.ts:59-77
Timestamp: 2026-03-17T05:02:59.105Z
Learning: In the Rezi project (`packages/core/src/widgets/`), `BreadcrumbProps.id` is intentionally optional. The widget implementation `createBreadcrumbWidgetVNode` auto-generates a stable internal ID via `ctx.id("breadcrumb")` when no explicit `id` is provided. Do not flag the optional `id` on `BreadcrumbProps` as a missing interactive-widget ID — the contract is fulfilled at the implementation level.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to packages/core/src/widgets/{ui.ts,types.ts,composition.ts,protocol.ts} : Treat `ui.ts`, `types.ts`, `composition.ts`, and `protocol.ts` as canonical widget factory, prop contracts, and callback signatures that must be preserved; these are safe modification zones
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to {packages/core/src/widgets/ui.ts,packages/core/src/widgets/types.ts} : Canonical API: preserve prop/callback names and signatures; these are the source of truth for widget contracts
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `defineWidget` for reusable stateful components that need composition context.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Read exploration order: packages/core/src/index.ts → packages/core/src/widgets/ui.ts → packages/core/src/widgets/types.ts → packages/core/src/widgets/composition.ts → packages/create-rezi/templates/ → packages/core/src/**/__tests__/
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Prefer semantic variants/intents over manual style-only distinctions for widget appearance.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-23T06:10:40.638Z
Learning: Applies to **/*.{ts,tsx} : Always use `ui.*` factory functions for widget construction (ui.button, ui.text, ui.panel, etc.) — never construct raw VNode objects with `{ kind: ..., props: ... }`
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to **/*.{ts,tsx} : Avoid: importing from internal paths instead of package exports; missing id on interactive widgets; conditional hook execution; in-place mutation of app state; duplicate widget IDs in one tree
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `ui.*` factories for widget construction instead of manually creating components.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use semantic widgets (`ui.status`, `ui.badge`, `ui.tag`, `ui.callout`, `ui.spinner`, `ui.progress`) instead of custom text-based status constructs.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-23T06:10:40.638Z
Learning: Applies to **/*.{ts,tsx} : Use `defineWidget()` for all reusable, stateful components — pass state via props, manage internal state with `ctx.useState()`
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-23T06:10:40.638Z
Learning: Applies to **/*.{ts,tsx} : All interactive widgets MUST have a unique `id` prop to ensure proper focus management and event routing
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Interactive widgets require unique `id` values within their scope.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-25T05:07:29.435Z
Learning: Applies to **/*.{ts,tsx} : All interactive widgets MUST have a unique `id` prop
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Every interactive widget must define a unique `id` within its scope; duplicate IDs are fatal.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-25T05:07:29.435Z
Learning: Applies to **/*.{ts,tsx} : Duplicate interactive widget IDs are fatal and will throw ZRUI_DUPLICATE_ID error
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Risk triage: docs-only updates (low risk) need lint/grep validation; widget prop renames (medium risk) need compile + affected tests + docs parity; runtime/router/reconcile/layout/drawlist changes (high risk) need full test suite + integration coverage + PTY evidence
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `accessibleLabel` when visual labels are ambiguous or missing from interactive widgets.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `ui.focusZone` for custom local navigation groups instead of manual focus management.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to packages/jsx/src/{components.ts,types.ts,createElement.ts,index.ts} : Maintain JSX parity with core widget API changes; update JSX components, types, and creation functions whenever core widget APIs change
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Keep layout tokens and theme tokens semantic; do not hardcode RGB/hex literals in app widgets.
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 216
File: scripts/drawlist-spec.ts:1-3
Timestamp: 2026-02-26T10:29:26.658Z
Learning: In the Rezi project, public API references and documentation must consistently use the current public protocol version (v1) after the reset, and must not reference the old v6 iteration. Review tests, scripts, and docs (including scripts/drawlist-spec.ts) to replace any v6 mentions with v1, and add a short note in release/docs where protocol versions are referenced to reflect the reset.
Comment on lines
+61
to
+70
| /** Modal width sizing. Supports fixed values, "auto", "full", `fluid(...)`, or `expr(...)`. */ | ||
| width?: SizeConstraintAtom; | ||
| /** Modal height sizing. Supports fixed values, "full", `fluid(...)`, or `expr(...)` (no "auto" height). */ | ||
| height?: Exclude<SizeConstraintAtom, "auto">; | ||
| /** Maximum width bound (cells, "full", `fluid(...)`, or `expr(...)`). */ | ||
| maxWidth?: Exclude<SizeConstraintAtom, "auto">; | ||
| /** Minimum width bound (cells, "full", `fluid(...)`, or `expr(...)`). */ | ||
| minWidth?: Exclude<SizeConstraintAtom, "auto">; | ||
| /** Minimum height bound (cells, "full", `fluid(...)`, or `expr(...)`). */ | ||
| minHeight?: Exclude<SizeConstraintAtom, "auto">; |
There was a problem hiding this comment.
Restore maxHeight in ModalProps.
This manual size surface mirrors width with minWidth/maxWidth, but height only exposes minHeight. In a split that is supposed to preserve the existing public contract, that asymmetry looks like an accidental omission and removes the non-fixed way to cap tall modal content. Based on learnings, "Canonical API: preserve prop/callback names and signatures; these are the source of truth for widget contracts."
Suggested fix
/** Minimum height bound (cells, "full", `fluid(...)`, or `expr(...)`). */
minHeight?: Exclude<SizeConstraintAtom, "auto">;
+ /** Maximum height bound (cells, "full", `fluid(...)`, or `expr(...)`). */
+ maxHeight?: Exclude<SizeConstraintAtom, "auto">;
/** Frame/surface colors for modal body and border. */
frameStyle?: OverlayFrameStyle;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/widgets/types/overlaysShell.ts` around lines 61 - 70, The
ModalProps type is missing a maxHeight mirror to maxWidth which breaks the
public contract; add a maxHeight?: Exclude<SizeConstraintAtom, "auto"> property
alongside height and minHeight in the ModalProps/overlaysShell definitions so
height bounds are symmetric (refer to the existing height, minHeight, maxWidth,
and minWidth properties to match typing and documentation).
Owner
Author
There was a problem hiding this comment.
Verified against origin/main: ModalProps does not currently expose maxHeight, so adding it here would expand the public API. Because this PR is intentionally scoped to file splitting with an unchanged surface, I’m leaving the modal contract as-is. If we want maxHeight, that should land in a follow-up behavior/API PR.
There was a problem hiding this comment.
@RtlZeroMemory, thanks for verifying against origin/main — that's the right call. My suggestion was based on an incorrect assumption that the symmetric maxHeight was part of the existing contract. Since the current API genuinely lacks it, the split correctly preserves the status quo and adding it here would be an unintended API expansion.
Would you like me to open a follow-up GitHub issue to track adding maxHeight?: Exclude<SizeConstraintAtom, "auto"> to ModalProps in a dedicated API PR?
🐇 …ears drooping slightly over a mistaken assumption…
🧠 Learnings used
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-23T06:10:40.638Z
Learning: Applies to packages/core/src/layout/**/*.ts : Layout engine uses constraint-based algorithm — use `ui.box` transition properties to control which layout properties animate (defaults to position, size, opacity; constrain with explicit `properties`)
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Prefer Layer 1 Widget API (`ui.*`) over Layer 2 Composition API for standard use cases, reserve Layer 4 VNode objects for low-level internals only.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to {packages/core/src/widgets/ui.ts,packages/core/src/widgets/types.ts} : Canonical API: preserve prop/callback names and signatures; these are the source of truth for widget contracts
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to packages/core/src/widgets/{ui.ts,types.ts,composition.ts,protocol.ts} : Treat `ui.ts`, `types.ts`, `composition.ts`, and `protocol.ts` as canonical widget factory, prop contracts, and callback signatures that must be preserved; these are safe modification zones
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `defineWidget` for reusable stateful components that need composition context.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Prefer semantic variants/intents over manual style-only distinctions for widget appearance.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to packages/jsx/src/{components.ts,types.ts,createElement.ts,index.ts} : Maintain JSX parity with core widget API changes; update JSX components, types, and creation functions whenever core widget APIs change
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Risk triage: docs-only updates (low risk) need lint/grep validation; widget prop renames (medium risk) need compile + affected tests + docs parity; runtime/router/reconcile/layout/drawlist changes (high risk) need full test suite + integration coverage + PTY evidence
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use semantic widgets (`ui.status`, `ui.badge`, `ui.tag`, `ui.callout`, `ui.spinner`, `ui.progress`) instead of custom text-based status constructs.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Use `ui.*` factories for widget construction instead of manually creating components.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Group sections with `ui.panel(...)` or `ui.card(...)` instead of manually styled containers.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to {docs/packages/jsx.md,packages/jsx/README.md,docs/getting-started/jsx.md} : Update JSX documentation to reflect API changes in core widgets
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Applies to **/*.{ts,tsx} : Avoid: importing from internal paths instead of package exports; missing id on interactive widgets; conditional hook execution; in-place mutation of app state; duplicate widget IDs in one tree
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-06T10:44:56.248Z
Learning: Applies to **/*.{ts,tsx} : Keep layout tokens and theme tokens semantic; do not hardcode RGB/hex literals in app widgets.
Learnt from: CR
Repo: RtlZeroMemory/Rezi PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-03T10:00:48.227Z
Learning: Read exploration order: packages/core/src/index.ts → packages/core/src/widgets/ui.ts → packages/core/src/widgets/types.ts → packages/core/src/widgets/composition.ts → packages/create-rezi/templates/ → packages/core/src/**/__tests__/
Learnt from: RtlZeroMemory
Repo: RtlZeroMemory/Rezi PR: 216
File: scripts/drawlist-spec.ts:1-3
Timestamp: 2026-02-26T10:29:26.658Z
Learning: In the Rezi project, public API references and documentation must consistently use the current public protocol version (v1) after the reset, and must not reference the old v6 iteration. Review tests, scripts, and docs (including scripts/drawlist-spec.ts) to replace any v6 mentions with v1, and add a short note in release/docs where protocol versions are referenced to reflect the reset.
This was referenced Mar 18, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packages/core/src/widgets/types.tsinto internal family modules underpackages/core/src/widgets/types/../widgets/types.jssurface unchangedWhy
Validation
npm run lintnpm run typechecknpm run buildnode scripts/run-tests.mjs --filter "packages/core/dist/widgets/__tests__/"node scripts/run-tests.mjs --filter "packages/jsx/dist/__tests__/parity"Summary by CodeRabbit